Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A handful of fixes #23

Merged
merged 4 commits into from
Jan 3, 2024
Merged

A handful of fixes #23

merged 4 commits into from
Jan 3, 2024

Conversation

akx
Copy link
Contributor

@akx akx commented Jan 3, 2024

  1. Check for Existing Contributions: There are none.
  2. Link Related Issues: There are none.
  3. Elaborate Your Changes: Please see below.
  4. Ultralytics Contributor License Agreement (CLA): I have contributed to other Ultralytics repositories already.

The commits, in order:

  1. This removes pyproject.toml, as this project is not a Python project that would require installation. (The pyproject.toml file also would not work, as it attempts a dynamic version configuration where no dynamic version configuration source is present.) It also removes the duplicated configuration from pyproject.toml.
  2. This changes the user attribution for the automated commits to Ultralytics Actions instead of @glenn-jocher. (git's user.name is not a GitHub username; it should be a realname.) This is good Git hygiene, not to mention that since this will evidently be used in Ultralytics's open-source projects that welcome external contributions, it sounds amiss that @glenn-jocher would be personally auto-formatting PRs.
  3. The action.yml file in this repository did not strictly adhere to the GitHub action schema; the type for inputs.default is string. (Your editor should highlight this as you edit the file; mine did.)
  4. Finally, there was a simple typo; while Tom Preston-Werner is indeed the author of the TOML file format, the extension isn't his first name. 😁

🛠️ PR Summary

Made with ❤️ by Ultralytics Actions

🌟 Summary

This PR updates defaults and enhances spelling checks in Ultralytics' GitHub Actions.

📊 Key Changes

  • Changed the defaults of Python, docstrings, markdown, and spelling inputs from Boolean false to String 'false'.
  • Updated the installation comment about the necessity of tomli for codespell.
  • Enhanced the codespell command to include a specified ignore-words list and files/directories to skip during the spell check.
  • Modified the git config to use a more generic Ultralytics Actions user for commits made by GitHub Actions.

🎯 Purpose & Impact

  • Ensuring input defaults are consistent and properly formatted as strings provides clarity and prevents potential confusion or errors in workflows.
  • Documentation improvements aid maintainers in understanding dependencies' purposes.
  • The customized codespell parameters reduce false positives by ignoring certain terms and skipping specified files, leading to more accurate spelling checks.
  • Changing the commit authorship to a generic user account makes automated actions distinct and unified across Ultralytics repositories, offering better traceability and consistency.

These changes enhance the automation of repository maintenance tasks, potentially improving code quality and reducing the manual workload on developers. 🚀

@akx
Copy link
Contributor Author

akx commented Jan 3, 2024

The fact that user.name is not a GitHub username is evident from e.g. the below squash commit:
Screenshot 2024-01-03 at 9 55 35

@akx
Copy link
Contributor Author

akx commented Jan 3, 2024

As alluded to in ultralytics/yolov5#12572 (comment), if the intent is that the target repository should configure these tools in its own pyproject.toml, then any configuration within the action YAML should be elided instead (and the action made to check that a pyproject.toml exists before it acts).

@glenn-jocher
Copy link
Member

glenn-jocher commented Jan 3, 2024

The fact that user.name is not a GitHub username is evident from e.g. the below squash commit: Screenshot 2024-01-03 at 9 55 35

@akx ah, I saw this also the other day, but I think this specific commit was when I experimented with removing my email from action.yml. Once I returned it GitHub seems to know it's me.

BTW, we have a bot account at https://github.com/ultralytics/UltralyticsAssistant with email [email protected], we probably want to use this account. Let me try to apply it and see what happens.

@glenn-jocher
Copy link
Member

@akx oh this is interesting, it seems like the action doesn't have all the required permissions to update PRs from forked repos. This could be a big problem since most PRs arrive that way.

@akx
Copy link
Contributor Author

akx commented Jan 3, 2024

ah, I saw this also the other day, but I think this specific commit was when I experimented with removing my email from action.yml. Once I returned it GitHub seems to know it's me.

Yes, GitHub maps commit identities to GitHub identities via email addresses (and you can make a commit look like it's been authored by anyone else lest you sign all of your own commits) – but in any case, it should be "Glenn Jocher", not glenn-jocher.

BTW, we have a bot account at @UltralyticsAssistant with email [email protected]

That would be much better, yes 😄

@glenn-jocher glenn-jocher merged commit 67b5103 into ultralytics:main Jan 3, 2024
1 check failed
@ultralytics ultralytics deleted a comment from pderrenger Apr 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants